Skip to content

Fixed assertion fail on resizing#2433

Open
oleg68 wants to merge 1 commit intoGrandOrgue:masterfrom
oleg68:bugfix/asset-image
Open

Fixed assertion fail on resizing#2433
oleg68 wants to merge 1 commit intoGrandOrgue:masterfrom
oleg68:bugfix/asset-image

Conversation

@oleg68
Copy link
Copy Markdown
Contributor

@oleg68 oleg68 commented Mar 4, 2026

Earlier when resising a panel lower it's original size there were assertion failed messages "Image is not valid".

The reason was that some elements had some size equal to 1 pixel. When downscaling the image, the size became 0 that made the bitmap as invalid.

This PR introduces lots of validations when scaling the bitmap. If some size becomes 0, then the GOBitmap instance is marked as invalid, and GetResultBitmap returns nullptr instead of invalid bitmap instance, and GODC::DrawBitmap does not try to draw it.

@oleg68 oleg68 requested review from larspalo and rousseldenis March 4, 2026 18:52
@larspalo
Copy link
Copy Markdown
Contributor

@oleg68 I'm not yet certain of how this approach will perform and affect some sample sets. There might be certain tricks done with transparent 1px images in a few sample sets. I'll need to test this carefully to see if there are side effects caused by it.

The scaling (both down and up) layout mechanism need a bit of re-designing anyway (for instance to assure that the keyboard layout always will be sane without disturbing blank spaces). Maybe rounding up and never going below 1 px might be a better idea than invalidating...

Copy link
Copy Markdown
Contributor

@larspalo larspalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my tests with sample sets that actually have 1px images (often transparent) there has been no issues so far with this PR. Something might of course surface in time. I've noted that some of the Sonus Paradisi sample sets have 1px images, but no conversion that I've tested has yet showed any issues. Thus I'm approving.

@oleg68
Copy link
Copy Markdown
Contributor Author

oleg68 commented Mar 27, 2026

@larspalo GrandOrgue built-in panels ex. Coupler or Metronome have some 1px elements.

@larspalo
Copy link
Copy Markdown
Contributor

GrandOrgue built-in panels ex. Coupler or Metronome have some 1px elements.

But is anything problematic? I've not seen it - but then again I might have missed something...

In principle, my earlier post still stands. There might be a better way to handle scaling/layout than the current version. But for now I'm ok with laying this to rest.

@oleg68
Copy link
Copy Markdown
Contributor Author

oleg68 commented Mar 27, 2026

GrandOrgue built-in panels ex. Coupler or Metronome have some 1px elements.

But is anything problematic?

image

I've not seen it - but then again I might have missed something...

Try to build GrandOrgue with debug, load the Demo Organ, open the Master Control panel and try to resize it below it's original size.

In principle, my earlier post still stands. There might be a better way to handle scaling/layout than the current version.

Earlier the 1px elements were never drown after resize, but GO stucked. My PR removes the stuck, but does not start drawing them.

There might be a better way to handle scaling/layout than the current version. But for now I'm ok with laying this to rest.

It is not so simple. So I cannot do it now. But your future PRs are welcome.

@larspalo
Copy link
Copy Markdown
Contributor

Try to build GrandOrgue with debug, load the Demo Organ, open the Master Control panel and try to resize it below it's original size.

Right, so this is actually a layout issue. Not just an image issue per se.

So I cannot do it now. But your future PRs are welcome.

Well, in time perhaps. The point is that a real fix is not than just hiding the issue... Since it only affects the debug builds anyway this PR is ok for now (I already approved of this PR, mind you). However, the layout issue behind this will likely require an actual fix somewhere down the line!

@oleg68
Copy link
Copy Markdown
Contributor Author

oleg68 commented Mar 31, 2026

@rousseldenis Could you approve this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants